Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impl ListIter for im::HashMap #1033

Closed
wants to merge 4 commits into from

Conversation

futurepaul
Copy link
Collaborator

This is motivated by my own need for a stable "key" of a list item so I can use that specific list item in another part of my app.

I'm guessing this is possible to do without im but it looks a lot more difficult so I will kindly ask not to be selected for that task :)

@futurepaul
Copy link
Collaborator Author

After going through the trouble of getting this compile with a new example, I'm realizing this example should probably just be an extension or modification of the existing list.rs example. thoughts?

@futurepaul futurepaul added S-needs-review waits for review enhancement adds or requests a new feature labels Jun 14, 2020
@xStrom
Copy link
Member

xStrom commented Jun 15, 2020

This was briefly mention on zulip and I tend to agree - HashMap doesn't seem like a natural fit for a List because the order is undefined - although stable in the current implementation. im::OrdMap would be a better fit for sure and would also help with your key requirement.

Example wise, yeah I'd say a strong no for another example just for the map. Every new example introduces maintenance burden. If there's a decent way to add it to the existing list example - that's cool. Otherwise I think just not having an example is fine too.

@cmyr
Copy link
Member

cmyr commented Jun 17, 2020

Agree that OrdMap is a better fit for this, otherwise this seems reasonable (outside of the fact that I want a better abstraction than ListIter for collections, but one thing at a time)

Also don't think this really needs a new example.

@xStrom xStrom added S-waiting-on-author waits for changes from the submitter and removed S-needs-review waits for review labels Jun 17, 2020
@futurepaul
Copy link
Collaborator Author

This makes a lot of sense @xStrom / @cmyr. Unfortunately I don't see the iter_mut method on im's OrdMap... might just be an oversight on their part?

@xStrom
Copy link
Member

xStrom commented Jun 19, 2020

That's interesting indeed. I asked it in im#138. Hopefully we'll know if this is something we could add.

@cmyr
Copy link
Member

cmyr commented Aug 22, 2020

@futurepaul going to close this for now, feel free to reopen if you pick it up again!

@cmyr cmyr closed this Aug 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement adds or requests a new feature S-waiting-on-author waits for changes from the submitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants